-
Notifications
You must be signed in to change notification settings - Fork 522
Exclude 'off' from fanMode capability for Thermostats #2673
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Exclude 'off' from fanMode capability for Thermostats #2673
Conversation
|
Invitation URL: |
Test Results 71 files 480 suites 0s ⏱️ Results for commit f0de5ed. ♻️ This comment has been updated with latest results. |
|
Minimum allowed coverage is Generated by 🐒 cobertura-action against f0de5ed |
c823475 to
f0de5ed
Compare
| supported_fan_modes_attribute = supported_fan_modes_capability.supportedAcFanModes | ||
| elseif device:supports_capability_by_id(capabilities.thermostatFanMode.ID) then | ||
| supported_fan_modes_attribute = capabilities.thermostatFanMode.supportedThermostatFanModes | ||
| supported_fan_modes_capability = capabilities.thermostatFanMode |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure I understand why this variable was introduced. Seems like it might be adding extra complexity without reason?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It was needed for the get_latest_state call in the logic down below. As we discussed, some of this logic won't be needed, so this variable can be removed
This change removes Off as a default value from supportedFanModes attribute for the Thermostat device type. Thermostats typically never support turning off the fan, but after switching from
thermostatFanModetofanModeit became an option.